Skip to content

Fix #2973: Check variances of class parents #4932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 26, 2018

Conversation

Blaisorblade
Copy link
Contributor

@Blaisorblade Blaisorblade commented Aug 11, 2018

Fix #2973: check variance of parent classes via ClassInfo.classParents (via @odersky's fix in #4057).
To account for @uncheckedVariance (unlike #4057), prevent Namer from stripping annotations in classParents. Surprisingly, the rest of Dotty doesn't mind!

We probably should test this on the new collections and the community build, since collections will stress this out much more.

@Blaisorblade Blaisorblade requested a review from odersky August 11, 2018 23:33
@Blaisorblade Blaisorblade self-assigned this Aug 11, 2018
@Blaisorblade Blaisorblade requested a review from smarter August 12, 2018 00:19
case _ => defn.ObjectType // can happen if class files are missing
}
}
tp.parents.map { p => transformedParent(apply(p)) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use { when ( works

@@ -0,0 +1,2 @@
class Foo[U]
class Bar[-T] extends Foo[T] // error: contravariant type T occurs in invariant position
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More testcases (related to annotations, etc) would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't find any annotation for parent classes apart from @uncheckedVariance. So added more tests for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just create a dummy annotation and use it wherever

@Blaisorblade Blaisorblade removed the request for review from odersky August 13, 2018 00:54
odersky and others added 3 commits August 26, 2018 03:53
@Blaisorblade Blaisorblade merged commit 724376e into scala:master Aug 26, 2018
@Blaisorblade Blaisorblade deleted the fix-#2973 branch August 26, 2018 11:45
Blaisorblade added a commit to dotty-staging/dotty that referenced this pull request Aug 26, 2018
Blaisorblade added a commit to dotty-staging/dotty that referenced this pull request Aug 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unsoundness due to missing variance checks for extends clause
3 participants